Repository navigation
fix: keep writes transactional after MySQL deadlock 1213 - #10651
paulbalandan merged 5 commits into
Conversation
|
Hi there, @KevinBlakeLopez! 👋 Thank you for sending this PR! We expect the following in all Pull Requests (PRs).
Important We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works. If pull requests do not comply with the above, they will likely be closed. Since we are a team of volunteers, we don't have any more time to work See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md |
4976289 to
7750791
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Prepared statements and failed transaction restarts can still allow subsequent writes to autocommit.
3 open issues
What changed in this PR
Addresses #10649 by tracking MySQLi transactions rolled back by deadlock error 1213, preventing later writes from autocommitting and avoiding false commit success. Reviewed under the assumed develop patch-release policy.
Changes:
- Adds rollback tracking, deferred transaction restart, and commit protection.
- Adds live MySQLi deadlock regression tests.
- Documents recovery behavior while leaving error 1205 unchanged.
Validation used targeted source inspection; live database tests and full validation remain with CI.
| File | Description |
|---|---|
| user_guide_src/source/database/transactions.rst | Documents MySQL deadlock recovery behavior. |
| user_guide_src/source/changelogs/v4.7.6.rst | Records the errno 1213 fix. |
| tests/system/Database/Live/MySQLi/DeadlockTransactionTest.php | Adds deadlock and subsequent-write regression tests. |
| system/Database/MySQLi/Connection.php | Detects deadlocks and restarts server transactions. |
| system/Database/BaseConnection.php | Tracks server rollback state and prevents false commit success. |
Review effort: Balanced
8089966 to
5cd609b
Compare
paulbalandan
left a comment
There was a problem hiding this comment.
Thank you! Please rebase to fix the conflicts.
InnoDB rolls the victim transaction back and mysqli::commit() still returns true. The next statement now starts a new server transaction, and transCommit() does not report success. Fixes codeigniter4#10649 Co-authored-by: Cursor <cursoragent@cursor.com>
mysqli_sql_exception::getCode() is already an int. Rector rejects the redundant cast. Co-authored-by: Cursor <cursoragent@cursor.com>
The restart BEGIN ran outside the query try block, after the restart flag was cleared. If the connection dropped after errno 1213, query(), ping() and reconnect() threw a raw mysqli_sql_exception, even with DBDebug false, and the next write skipped BEGIN. BEGIN now runs inside the try, and the restart stays pending until it succeeds. transCommit() returned false and left transDepth at 1 with the restarted server transaction open. A caller that retried without transRollback() nested inside it, got true from transCommit(), and nothing was committed. transCommit() now rolls that transaction back before returning false. The guide and changelog now limit the fix to query(), simpleQuery() and the Query Builder. Prepared queries are not covered. The deadlock test records the probe table before recreating it, so teardown drops it. Refs codeigniter4#10649 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With mysqli exception reporting off, begin_transaction() returns false instead of throwing. The restart flag was then cleared and the statement ran without a transaction. execute() now returns false and keeps the restart pending. The guide now says the rollback happens when the outermost transaction finishes, and that a nested transCommit() still returns true. Drop the issue link from the changelog entry to match the changelog style. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The flags are MySQLi-only, so BaseConnection stays unchanged. _transBegin() and _transRollback() clear them after success. When InnoDB has already rolled back, _transCommit() calls transRollback() and returns false. Co-authored-by: Cursor <cursoragent@cursor.com>
72aab61 to
1fdc272
Compare
|
Thank you! |


Description
Fixes #10649.
InnoDB errno 1213 rolls the victim transaction back. CodeIgniter left
transDepthat 1, so the next write on that connection autocommitted.transComplete()returned false and did not remove that write.transCommit()calledmysqli::commit(), which returns true, and reported success while the victim row was already gone.This is only errno 1213. Errno 1205 is not handled here, because
innodb_rollback_on_timeoutdecides whether InnoDB rolled that transaction back.After 1213, the next statement on that connection starts a new server transaction, so it does not autocommit.
transCommit()returns false without callingmysqli::commit().transComplete()still sees the failed status and rolls the new server transaction back.On
develop(df8b96bee) the new live tests failed before this change.transCommit():transComplete()already returned false, and the following row stayed visible:The victim id 101 was absent in both runs. After the change,
DeadlockTransactionTestpasses, and the existingTransactionTest,TransactionDBDebugTrueTest, andTransactionDBDebugFalseTeststill pass (local MySQL 8.0.45, PHP 8.5.12, MySQLi).Checklist:
Made with Cursor